Skip to content

in_calyptia_fleet: windows support.#7929

Merged
edsiper merged 7 commits intomasterfrom
pwhelan-fleet-windows
Sep 21, 2023
Merged

in_calyptia_fleet: windows support.#7929
edsiper merged 7 commits intomasterfrom
pwhelan-fleet-windows

Conversation

@pwhelan
Copy link
Contributor

@pwhelan pwhelan commented Sep 15, 2023

Summary

This PR implemenets hot reload in windows for the in_calyptia_fleet plugin. It includes the necessary changes for the plugin itself as well as rudimentary support for hot-reload under win32.

Obstacles

The calyptia fleet plugin reloads configuration taken from calyptia, which requires hot-reload in windows. This requires two major changes:

Solutions

TLS

The first concern was fixed in monkey and is still awaiting merging upstream to fluent-bit.

SIGHUP

I used GenerateConsoleCtrlEvent to simulate the behavior of the SIGHUP signal in windows and implemented a simple mechanism to signal the main thread from the GenerateConsoleCtrlEvent handler. This is so the main thread can execute flb_reload.

The implementation I went with uses a simple int to mark that the signal has been raised. The main loop then checks it, marks it as zero and then executes flb_reload. This is, admittedly, functional but not elegant.

Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • Example configuration file for the change
  • Debug log output from testing the change
  • Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • Run local packaging test showing all targets (including any new ones) build.
  • Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • Documentation required for this feature

Backporting

  • Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

@pwhelan pwhelan temporarily deployed to pr September 15, 2023 14:51 — with GitHub Actions Inactive
@pwhelan pwhelan temporarily deployed to pr September 15, 2023 14:51 — with GitHub Actions Inactive
edsiper
edsiper requested changes Sep 15, 2023
View reviewed changes
@pwhelan pwhelan temporarily deployed to pr September 15, 2023 15:51 — with GitHub Actions Inactive
@pwhelan pwhelan temporarily deployed to pr September 15, 2023 15:51 — with GitHub Actions Inactive
@pwhelan pwhelan force-pushed the pwhelan-fleet-windows branch from b2234f8 to 23942ed Compare September 15, 2023 16:14
@pwhelan pwhelan temporarily deployed to pr September 15, 2023 16:15 — with GitHub Actions Inactive
@pwhelan pwhelan temporarily deployed to pr September 15, 2023 16:15 — with GitHub Actions Inactive
@pwhelan pwhelan temporarily deployed to pr September 15, 2023 17:09 — with GitHub Actions Inactive
@pwhelan pwhelan temporarily deployed to pr September 15, 2023 17:09 — with GitHub Actions Inactive
@pwhelan pwhelan temporarily deployed to pr September 15, 2023 17:09 — with GitHub Actions Inactive
@pwhelan pwhelan temporarily deployed to pr September 15, 2023 17:35 — with GitHub Actions Inactive
@patrick-stephens patrick-stephens added the ok-package-test Run PR packaging tests label Sep 15, 2023
@niedbalski niedbalski requested a review from edsiper September 18, 2023 09:54
@pwhelan
Copy link
Contributor Author

pwhelan commented Sep 18, 2023

There is still a blocker concerning the compile error on 32 bit windows which I have yet to resolve.

@patrick-stephens
Copy link
Contributor

patrick-stephens commented Sep 18, 2023

There is still a blocker concerning the compile error on 32 bit windows which I have yet to resolve.

This one? https://ci.appveyor.com/project/fluent/fluent-bit-2e87g/builds/48047875/job/ubcx7x6cxx26ftkj
That looks like the usual flaky test...

[ FAILED ]
  log.c:16: Check (*interval >= timeout - 1) && *interval <= timeout... failed
    interval error. got=3 expect=4-5
  log.c:92: Check ret == 0... failed
    update_and_check_interval for TEST_RECORD_01 failed. i=9
  log.c:16: Check (*interval >= timeout - 1) && *interval <= timeout... failed
    interval error. got=3 expect=4-5
  log.c:98: Check ret == 0... failed
    update_and_check_interval for TEST_RECORD_02 failed. i=9
Test cache_one_slot...                          
[ OK ]
FAILED: 1 of 2 unit tests has failed.

@pwhelan
Copy link
Contributor Author

pwhelan commented Sep 18, 2023

It was failing on compile before. Seems like my last commit fixed it.

@edsiper
Copy link
Member

edsiper commented Sep 18, 2023

@pwhelan I merged #7925 which generated some conflicts here, please fix the conflicts.

@pwhelan
fluent-bit: use console ctrl handlers in win32 to simulate SIGHUP.
5fbda04 
Signed-off-by: Phillip Whelan <phil@calyptia.com>
@pwhelan
in_calyptia_fleet: reenable fleet on windows.
b65f87c 
Signed-off-by: Phillip Whelan <phil@calyptia.com>
@pwhelan
in_calyptia_fleet: use GenerateConsoleCtrlEvent on win32 to execute r…
2e83883 
…eload.

Signed-off-by: Phillip Whelan <phil@calyptia.com>
@pwhelan
in_calyptia_fleet: add support for creating configuration files for w…
1fccdfa 
…in32.

Signed-off-by: Phillip Whelan <phil@calyptia.com>
@pwhelan
fluent-bit: fix indentation.
4715697 
Signed-off-by: Phillip Whelan <phil@calyptia.com>
@pwhelan
fluent-bit: follow PHANDLER_ROUTINE definition more closely.
51f6b9c 
Signed-off-by: Phillip Whelan <phil@calyptia.com>
@pwhelan
Copy link
Contributor Author

pwhelan commented Sep 20, 2023 •
edited
Loading

I have resolved the conflicts and added a wrapper for readlink for win32. (I opened the PR #7951 to merge the fix).

The commit 59270d8 from monkey still needs to be integrated before windows reload can work. Without the calyptia fleet plugin will crash during co-routine setup.

@pwhelan
in_calyptia_fleet: add readlink equivalent for windows.
141c3c2 
Signed-off-by: Phillip Whelan <phil@calyptia.com>
@pwhelan pwhelan force-pushed the pwhelan-fleet-windows branch from 02d3a2c to 141c3c2 Compare September 20, 2023 14:50
@pwhelan pwhelan temporarily deployed to pr September 20, 2023 14:51 — with GitHub Actions Inactive
@pwhelan pwhelan temporarily deployed to pr September 20, 2023 14:51 — with GitHub Actions Inactive
@pwhelan pwhelan temporarily deployed to pr September 20, 2023 14:51 — with GitHub Actions Inactive
@pwhelan pwhelan mentioned this pull request Sep 20, 2023
Merged
7 tasks
@pwhelan pwhelan temporarily deployed to pr September 20, 2023 15:20 — with GitHub Actions Inactive
@edsiper edsiper merged commit 0f1f3e6 into master Sep 21, 2023
@edsiper edsiper deleted the pwhelan-fleet-windows branch September 21, 2023 14:26
@Porkepix Porkepix mentioned this pull request Sep 28, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fujimotos fujimotos Awaiting requested review from fujimotos

@niedbalski niedbalski Awaiting requested review from niedbalski niedbalski is a code owner

@patrick-stephens patrick-stephens Awaiting requested review from patrick-stephens patrick-stephens is a code owner

@celalettin1286 celalettin1286 Awaiting requested review from celalettin1286 celalettin1286 is a code owner

@leonardo-albertovich leonardo-albertovich Awaiting requested review from leonardo-albertovich

@koleini koleini Awaiting requested review from koleini

@edsiper edsiper Awaiting requested review from edsiper edsiper is a code owner

Assignees

No one assigned

Labels

docs-required ok-package-test Run PR packaging tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pwhelan @patrick-stephens @edsiper